Skip to content

[STALE] add SCSI Protocol support #1155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 12 commits into from
Closed

[STALE] add SCSI Protocol support #1155

wants to merge 12 commits into from
+587 −3

Conversation

sky5454
Copy link
Contributor

@sky5454 sky5454 commented May 1, 2024 •
edited by phip1611
Loading

Steps to Undraft

  • Restart the work (PR ist stale for over 1 year now)

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

Only preliminary support.
some problem I faced:

  • the buffer of I/O data. maybe the best way is to wrap by Reader/Writer trait
    • uefi-raw/src/protocol/scsi.rs: ScsiIoProtocol::execute_scsi_command()
    • uefi/src/proto/scsi.rs: ExtScsiPassThru::pass_thru()
    • wrap and make the ScsiIoScsiRequestPacket/ExtScsiIoScsiRequestPacket to be conveniently used in Rust
  • The dev path which using the Device Path Protocol
  • TARGET_MAX_BYTES unused, for: The Target is an array of size TARGET_MAX_BYTES and it represents the id of the SCSI device to send the SCSI Request Packet.
  • The naming rule of enum.
  • The data sync with c and rust ffi, vec <-> c data ptr and len.
#[derive(Debug)]
#[repr(C)]
pub struct ScsiIoScsiRequestPacket {
    pub timeout: u64,

  /// DataBuffer: A pointer to the data buffer to transfer from or to the SCSI device.
  ///  - InDataBuffer: A pointer to the data buffer to transfer between the SCSI controller and the SCSI device for SCSI READ command. For all SCSI WRITE Commands this must point to NULL .
  ///  - OutDataBuffer: A pointer to the data buffer to transfer between the SCSI controller and the SCSI device for SCSI WRITE command. For all SCSI READ commands this field must point to NULL .
    


    pub in_data_buffer: *mut c_void,
    pub out_data_buffer: *mut c_void,
    pub sense_data: *mut c_void,
    pub cdb: *mut c_void,

    pub in_transfer_length: u32,
    pub out_transfer_length: u32,

    pub cdb_length: u8,
    pub data_direction: DataDirection,
    pub host_adapter_status: HostAdapterStatus,
    pub target_status: TargetStatus,
    pub sense_data_length: u8,
}

#[derive(Debug)]
#[repr(C)]
pub struct ExtScsiIoScsiRequestPacket {
    pub timeout: u64,

    pub in_data_buffer: *mut c_void,
    pub out_data_buffer: *mut c_void,
    pub sense_data: *mut c_void,
    pub cdb: *mut c_void,

    pub in_transfer_length: u32,
    pub out_transfer_length: u32,

    pub cdb_length: u8,
    pub data_direction: ExtDataDirection,
    pub host_adapter_status: ExtHostAdapterStatus,
    pub target_status: ExtTargetStatus,
    pub sense_data_length: u8,
}

After read "SPC" which docs from T10 (SCSI Standard Groups), I Know that in_data_buffer,out_data_buffer,sense_data,cdb have variable length. so I decide to make a struct like

    pub timeout: u64,
    /// data_buffer is in_data_buffer or out_data_buffer
    pub data_buffer: Vec<u8>
    pub cdb: Vec<u8>
    pub sense_data: Vec<u8>
    pub data_direction: ExtDataDirection,
    pub host_adapter_status: ExtHostAdapterStatus,
    pub target_status: ExtTargetStatus,

but data couldn't share between FFI and c. That's a problem. We should share those data by ptr.

according to uefi scsi usage from edk2:

nicholasbishop
nicholasbishop reviewed May 4, 2024
View reviewed changes
sky5454 added 2 commits May 4, 2024 20:38
@sky5454
[SCSI] use newtype_enum instead of bitflags.
b36ba06
@sky5454
[SCSI] refactor and add test. fixed ExtScsiPassThruProtocol::mode; no…
ea3e4b1 
…w `execute_scsi_command` send req packet is ok.

**PASSED**:
 - SCSI_IO all api, SCSI_THRU `mode()` and `reset()` api, are test passed.
**PROBLEM**:
 1. panic at allocator.rs while test_scsi_io return;
 2. still problem with `DevicePath`,`target_id`,`pass_thru`;
**UNKNOWN**:
 - scsi req ok but resp without test at `execute_scsi_command()`.
@sky5454
Copy link
Contributor Author

sky5454 commented May 5, 2024 •
edited
Loading

SCSI_IO

The main part of SCSI_IO is finished. though something need some optimization.

  • data_buf maybe need in and out at the sametime.?
  • resp no test still.

Running at VMWare with two SCSI sample TestUnitReady and InquiryCommand:
image

EXT_SCSI_THRU

only test mode() and reset() passed.
some trouble.

  • DevicePath/DevicePathProtocol
  • Target ids
  • the packet sample about scsi thru. I couldn't find it clearly.

image

@sky5454 sky5454 marked this pull request as ready for review May 11, 2024 03:04
@sky5454
Copy link
Contributor Author

sky5454 commented May 11, 2024 •
edited
Loading

  • Now UEFI SCSI I/O protocol is ready.
  • And UEFI Scsi Thru I/O protocol Will not be published in this PR. I would continue to code it at branch sky5454:dev-scsi

image

after you review the code and resolve the panic, you could merge it.

@sky5454 sky5454 requested a review from nicholasbishop May 11, 2024 05:50
@sky5454 sky5454 mentioned this pull request May 11, 2024
Closed
@nicholasbishop
Copy link
Member

Before I review fully, it would help to do some cleanup:

  • Fix CI errors (run formatting, fix lints, etc)
  • Remove all unused code
  • Resolve TODOs if possible

This will make review easier :)

@sky5454
Copy link
Contributor Author

sky5454 commented Dec 29, 2024

sry, has no time to cleanup it for my tired life, just review/reedit and merge it plz.

@nicholasbishop nicholasbishop mentioned this pull request Jan 11, 2025
Merged
2 tasks
@phip1611
Copy link
Member

The split-out from #1517 took care of the uefi-raw part. Someone needs to rebase it and do the remaining work for uefi (high-level wrapper)

@phip1611 phip1611 changed the title add SCSI Protocol support [STALE] add SCSI Protocol support Jun 22, 2025
@phip1611 phip1611 marked this pull request as draft June 22, 2025 10:36
@phip1611
Copy link
Member

sry, has no time to cleanup it for my tired life, just review/reedit and merge it plz.

Giving this information, I'm in favor of closing this for better PR hygiene in this repository.

We are very happy and open for future contributions that restart/continue the work tho! :)

@phip1611 phip1611 closed this Oct 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nicholasbishop nicholasbishop Awaiting requested review from nicholasbishop

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sky5454 @nicholasbishop @phip1611